I think we don't need to tell students to use method overloading - the exercise has been designed so that the GameMaster.describe must be overloaded. So, all passing solutions should already be using method overload to pass.

Are you certain?

Is it not possible to write the solution like:

public String describe(Object...arguments) {
  // ...
}

...because if it is, I guarantee you, eventually someone will as overloading does not exist in all programming languages!

Oh man, I didn't think of that! Sorry @chiarazarrella, but I think @SleeplessByte is right, so we could keep this one.

Update: Actually, I just tested this public String describe(Object...arguments) solution on the platform. Doing causes the tests to fail with:

Message: Please implement the 'describe(Character character) method
Exception: org.opentest4j.AssertionFailedError: Please implement the 'describe(Character character) method
	at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502)
	at GameMasterTest.implementedDescribeCharacter(GameMasterTest.java:15)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

There are specific tests that check that the methods are overloaded and public.

FWIW: I had not checked the tests 😁

Looking at your tests for the "not use", "partial" and "use string format not use" method overloading, I'm a little confused into how method overloading isn't being used in their samples because the describe method is overloaded (defined multiple times, with different parameter types). Did you mean reuse method?

Oh! I see; what I meant was to exploit method overloading and yes, it is done indeed by reusing the methods.

However, I have noticed that it already exists something similar, the reuse_code comment. Should this be used instead? Or is it enough to only modify the name of the comment from use_method_overloading to reuse_method?

I think, if we can, we should use the same reuse_code comment. The comment needs to be specific about which describe method it should use though because all the methods are named describe. I'd suggest including the parameter's type in the comment. For example:

The describe(Character, Destination) method should reuse the logic implemented in describe(Character, Destination, TravelMethod). Reusing existing methods can help make code easier to maintain.

The methods can be passed as parameters to the comment (see AnalyzerIntegrationTest.loglevels.NoReuseOfBothMethods.approved.txt for an example).

Okay. I have a little doubt about the following, from the design.md:
If the student does not reuse the method describe(Character), describe(Destination), describe(TravelMethod) or describe(Character, Destination, TravelMethod) to solve the describe(Character, Destination) method, instruct them to do so.

Should we provide two comments? or could we modify the comment to inject also a sentence? Because I have seen that the reuse_code comment takes as input a string, we could manage to create a comment like this:

The describe(Character, Destination) method should reuse the logic implemented in describe(Character, Destination, TravelMethod) or in the individual methods describe(Character), describe(Destination), describe(TravelMethod). Reusing existing methods can help make code easier to maintain.

This means that the parameter acting as %<methodToCall>s will be a whole sentence:
describe(Character, Destination, TravelMethod) or in the single methods describe(Character), describe(Destination), describe(TravelMethod).

I think using two comments is fine and is the easier option. If we change the parameter to allow sentences as a parameter, we should check that the comments made in still make sense (because the parameter is enclosed in back ticks in the markdown).

The other possibility is to use a generic comment that can cover all the cases. For example:

The describe(Character, Destination) method should reuse the logic implemented in describe(Character, Destination, TravelMethod), describe(Character), describe(Destination) or describe(TravelMethod). Reusing existing methods can help make code easier to maintain.

We should handle the backticks even with two comments, though. Especially, if we need to keep the highlight of the methods only.

For example, with the following overloaded method describe(Character, Destination, TravelMethod), the comment would be:

The describe(Character, Destination, TravelMethod) method should reuse the logic implemented in describe(Character), describe(Destination) and describe(TravelMethod). Reusing existing methods can help make code easier to maintain.

and the injected string would be with the handle of the backticks:

describe(Character)`, `describe(Destination)`, and `describe(TravelMethod)

Oh, I see what you mean! I think that could work, but I'd lean towards making a copy of the comment specifically for this case. For example, the entire comment would be (no parameters in the comment):

The `describe(Character, Destination, TravelMethod)` method should reuse the logic implemented in `describe(Character)`, `describe(Destination)` and `describe(TravelMethod)`. Reusing existing methods can help make code easier to maintain.

If we close and re-open back ticks in the parameters and someone make changes to the comment's format in the future, they will need to be aware of the usage here. For example, if the change was to make methods italic, like describe(Character), the comment would look like this:

The _`%<callingMethod>s`_ method should reuse the logic implemented in _`%<methodToCall>s`_.
Reusing existing methods can help make code easier to maintain.

We'd need to remember to update the parameter accordingly, otherwise the commas and the word and would also be in italics.

describe(Character)`_, _`describe(Destination)`_, and _`describe(TravelMethod)

If we make a copy of the comments with the hard coded methods, we still need to remember to update the copy. But, if we forget to update it, at least the formatting wouldn't look weird for the comment.

Okay, I agree on that!
Then, I will create these two specific comments and update you asap when everything is done. :)

I will push the hardcoded comments on the web-copy repository when I will receive the okay on this one. 😄

Drive-by review from a guardian: we use Git attribution and config.json attribution. Highly recommend not adding @author tags as it will lead to bikeshedding when to add/remove/ping people in the future.

Okay, sorry! I got it done automatically, didn't even notice it. I will remove it :)

Are you certain?

Is it not possible to write the solution like:

public String describe(Object...arguments) {
  // ...
}

...because if it is, I guarantee you, eventually someone will as overloading does not exist in all programming languages!

Could we determine which method is being visited more accurately? My concern with simply counting the number of parameters is that it could also match other describe methods that students might write but we don't anticipate. For example:

public class GameMaster {

        public String describe(Character character) {
            // ...
        }

        public String describe(Destination destination) {
            // ...
        }

        public String describe(TravelMethod travelMethod) {
            // ...    
        }

        public String describe(Character character, Destination destination, TravelMethod travelMethod) {
            // ... 
        }

        public String describe(Character character, Destination destination) {
            // ...
        }

        private String describe(int someNumber, String someString) {
            // Also has two parameters!
        }
}

I think one way is to have the analyzer check the parameter's types (e.g. node.getParameters().stream().map(Parameter::getTypeAsString)).

I though this kind of check was already done by the tests for passing the exercise. But I can modify it by checking also the type of parameters :)

-- pushed the new changes

The tests only check that the ones that are needed are there (e.g. the public ones in above). It can't check if there are any extra unexpected ones (like the private one in my example above).

What does "Hardcoded" in the name refer to?

It refers to the fact of using the same comment of "Reuse_code", but with the parameters "hardcoded", as we said here.
You can check this PR for website-copy.

I think I can see where you're coming from, but it might not be clear to what's "hardcoded". Could we come up with another name? The only alternative suggestions I could think was ReuseCodeFromDescribeTwoParams and ReuseCodeFromDescribeThreeParams.

Suggest using more descriptive names for some of the variables, for example:

• paramTypes for params
• DESTINATION_TYPE for DESTINATION
• CHARACTER_TYPE for CHARACTER
• TRAVEL_METHOD_TYPE for TRAVEL_METHOD

When I first read this method, I had initially thought params was the list of parameter names and DESTINATION was the name of the destination variable and CHARACTER was name of the character variable. It took me a while they were actually the names of the types (hence the suggestion).

I see. You are right! I really need to think more about it! I usualy take this for granted, but thank you. I guess this is what you can learn from working on open source project. 😄

The tests only check that the ones that are needed are there (e.g. the public ones in above). It can't check if there are any extra unexpected ones (like the private one in my example above).

I think I can see where you're coming from, but it might not be clear to what's "hardcoded". Could we come up with another name? The only alternative suggestions I could think was ReuseCodeFromDescribeTwoParams and ReuseCodeFromDescribeThreeParams.